Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FINE] Porting newly supported Tower credential types to Fine #15780

Merged
merged 7 commits into from
Nov 28, 2017

Conversation

jameswnl
Copy link
Contributor

@jameswnl jameswnl commented Aug 9, 2017

Backporting via cherrypick proved to be complicated with too many conflicts. Instead this PR is created to port the feature to Fine and that effectively contains the following

The following are original PRs for masters (manageiq repo and manageiq-provider-ansible_tower repo) and their corresponding commits in this PR.

master PRs corresponding commits
Re-record vcr cassette and spec update, plus a script to generate Tower objects re-record tower refresher cassette
New Tower credential types support other credential types of Tower
Azure Classic credential added / Specs added for azure-classic-credential add azure_classic_credential, plus spec update
Backport? (copy) the applicable parts of "New Tower credential types" / Azure Classic Credential added for embedded Ansible add newly supported Tower credentials to embedded_ansible

The following 2 PRs are 1-liners that are included in the commits.

@miq-bot miq-bot changed the title porting spec_helper.rake to fine [FINE] porting spec_helper.rake to fine Aug 9, 2017
@simaishi simaishi requested a review from blomquisg August 9, 2017 18:08
@simaishi simaishi self-assigned this Aug 9, 2017
@jameswnl jameswnl changed the title [FINE] porting spec_helper.rake to fine [FINE] Porting newly supported Tower credential types to Fine Aug 10, 2017
@jameswnl jameswnl changed the title [FINE] Porting newly supported Tower credential types to Fine [WIP] [FINE] Porting newly supported Tower credential types to Fine Aug 10, 2017
@jameswnl
Copy link
Contributor Author

@miq-bot add_label wip

@miq-bot miq-bot added the wip label Aug 10, 2017
@jameswnl jameswnl changed the title [WIP] [FINE] Porting newly supported Tower credential types to Fine [FINE] Porting newly supported Tower credential types to Fine Aug 14, 2017
@jameswnl
Copy link
Contributor Author

@miq-bot remove_label wip

@miq-bot miq-bot removed the wip label Aug 14, 2017
Copy link
Member

@durandom durandom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@agrare agrare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jameswnl Its realistically impossible to confirm that +13,656 −3,439 worth of changes is the same as in master

Can you try to at do these one PR at a time even if not cherry-picked?

@durandom
Copy link
Member

I was glancing at all PRs listed in the description and had a quick look if they are included in here. So at least I tried to check :) But I'm with @agrare that it's not really 100% sure.

@jameswnl
Copy link
Contributor Author

@agrare @durandom my PR have commits that (roughly) correspond to the original PRs. I've updated the PR description with the mappings.

(It's not an exact match because of the split of repos after Fine.)

Copy link
Member

@durandom durandom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@agrare as said in my previous comment, I looked at all individual commits and glanced over the corresponding PRs - so I'm 👍 with this PR as is

Copy link
Member

@agrare agrare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍
ping @simaishi

@simaishi
Copy link
Contributor

Since this isn't a blocker, I'll get this in later when we start working on the next release.

@durandom
Copy link
Member

Since this isn't a blocker, I'll get this in later when we start working on the next release.

@jameswnl prepare for more rebasing 😆

@jameswnl jameswnl closed this Nov 2, 2017
@jameswnl jameswnl deleted the cred-fine branch November 2, 2017 17:09
@jameswnl jameswnl restored the cred-fine branch November 2, 2017 17:21
@jameswnl jameswnl reopened this Nov 2, 2017
@miq-bot
Copy link
Member

miq-bot commented Nov 2, 2017

Some comments on commits jameswnl/manageiq@4305c94~...95767b1

lib/tasks_private/spec_helper.rake

  • ⚠️ - 114 - Detected puts. Remove all debugging statements.
  • ⚠️ - 189 - Detected puts. Remove all debugging statements.
  • ⚠️ - 195 - Detected puts. Remove all debugging statements.
  • ⚠️ - 205 - Detected puts. Remove all debugging statements.
  • ⚠️ - 66 - Detected puts. Remove all debugging statements.
  • ⚠️ - 74 - Detected puts. Remove all debugging statements.

@miq-bot
Copy link
Member

miq-bot commented Nov 2, 2017

Checked commits jameswnl/manageiq@4305c94~...95767b1 with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0
33 files checked, 54 offenses detected

app/models/manageiq/providers/ansible_tower/shared/automation_manager/azure_credential.rb

  • ❗ - Line 10, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 22, Col 5 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 28, Col 5 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 34, Col 5 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 3, Col 5 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 4, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 5, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 8, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 9, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.

app/models/manageiq/providers/ansible_tower/shared/automation_manager/google_credential.rb

  • ❗ - Line 13, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 14, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 15, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 16, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 19, Col 5 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.

app/models/manageiq/providers/ansible_tower/shared/automation_manager/network_credential.rb

  • ❗ - Line 17, Col 5 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 18, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 19, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 20, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 23, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 24, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 25, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 27, Col 5 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 28, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 29, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 30, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 31, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 33, Col 5 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 34, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 35, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 36, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 37, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 3, Col 5 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.

app/models/manageiq/providers/ansible_tower/shared/automation_manager/openstack_credential.rb

  • ❗ - Line 17, Col 5 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 31, Col 5 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 3, Col 5 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.

app/models/manageiq/providers/ansible_tower/shared/automation_manager/rackspace_credential.rb

  • ❗ - Line 3, Col 5 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.

app/models/manageiq/providers/ansible_tower/shared/automation_manager/satellite6_credential.rb

  • ❗ - Line 10, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 11, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 12, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 3, Col 5 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 4, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 5, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 6, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
  • ❗ - Line 9, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.

app/models/manageiq/providers/ansible_tower/shared/inventory/parser/automation_manager.rb

lib/tasks_private/spec_helper.rake

  • 💣 💥 🔥 🚒 - Line 101, Col 39 - Syntax - unexpected token tIDENTIFIER
    (Using Ruby 2.2 parser; configure using TargetRubyVersion parameter, under AllCops)
  • 💣 💥 🔥 🚒 - Line 109, Col 25 - Syntax - unexpected token tCONSTANT
    (Using Ruby 2.2 parser; configure using TargetRubyVersion parameter, under AllCops)
  • 💣 💥 🔥 🚒 - Line 111, Col 10 - Syntax - unexpected token klEND
    (Using Ruby 2.2 parser; configure using TargetRubyVersion parameter, under AllCops)
  • 💣 💥 🔥 🚒 - Line 84, Col 20 - Syntax - unexpected token tLSHFT
    (Using Ruby 2.2 parser; configure using TargetRubyVersion parameter, under AllCops)
  • 💣 💥 🔥 🚒 - Line 85, Col 10 - Syntax - unexpected token klBEGIN
    (Using Ruby 2.2 parser; configure using TargetRubyVersion parameter, under AllCops)
  • 💣 💥 🔥 🚒 - Line 88, Col 58 - Syntax - unexpected token tCONSTANT
    (Using Ruby 2.2 parser; configure using TargetRubyVersion parameter, under AllCops)
  • 💣 💥 🔥 🚒 - Line 92, Col 65 - Syntax - unexpected token tIDENTIFIER
    (Using Ruby 2.2 parser; configure using TargetRubyVersion parameter, under AllCops)
  • 💣 💥 🔥 🚒 - Line 97, Col 6 - Syntax - unexpected token tCONSTANT
    (Using Ruby 2.2 parser; configure using TargetRubyVersion parameter, under AllCops)

spec/lib/miq_automation_engine/service_models/miq_ae_service_manageiq-providers-ansible_tower-automation_manager-azure_classic_credential_spec.rb

  • ❗ - Line 1, Col 1 - Style/FileName - The name of this source file (miq_ae_service_manageiq-providers-ansible_tower-automation_manager-azure_classic_credential_spec.rb) should use snake_case.

@simaishi simaishi closed this Nov 20, 2017
@simaishi simaishi reopened this Nov 20, 2017
@cben
Copy link
Contributor

cben commented Nov 28, 2017

Travis errors are the ones fixed by #16531, if you close/open it should pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants